-
Notifications
You must be signed in to change notification settings - Fork 3
[feat] 게임 시작 후 게임 진행 + 타이머 기능 추가 #94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
||
| private String getDestination(Long roomId) { | ||
| return "/sub/room/" + roomId; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getDestinatnion을 public static 으로 변경해서 util로 분리하는건 어떨까요?
현재 roomService, gameService에서 중복인 메소드라서요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
util 분리가 좋을 것 같습니다 ! 수정하겠습니다 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
흠.. 얘도 WebSocketUtils에서 관리하면 될까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
우선은 WebSocketUtils로 분리해두었습니다 !
| room.increaseCurrentRound(); | ||
|
|
||
| // 타이머 추가하기 | ||
| timerService.startTimer(room, CONTINUE_DELAY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
261 라인부터 272라인까지 혹시 아래 if문 안에서 실행되는건가요?
if (answer.equals(chatMessage.message())) {
현재로선 메세지 send 후 } 중괄호가 끝난 것 처럼 보입니다!
정답 처리시에만 increase 및 MessageSend.send를 실행하게 해놓아서 타이머 로직 또한 이 if문에 포함되어야하지 않을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
헉! 리팩토링 전 코드로 return문이 실행된다고 생각했네요..!! 감사합니다 ! 이 부분도 수정하겠습니다 ! :)
| handleTimeout(room); | ||
| }, | ||
| delaySec + room.getGameSetting().getTimeLimit(), | ||
| TimeUnit.SECONDS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오홍.. 재연결 유예시간 로직 구현할 때 참고해서 구현해봐야겠군요 👀
|
|
||
| private String getDestination(Long roomId) { | ||
| return "/sub/room/" + roomId; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기도 있군요.
확실히 Util로 분리해야할것 같습니다.. 🥕
|
|
||
| private Long quizId; | ||
| private Integer round; // 게임 변경 시 해당 게임의 총 문제 수로 설정 | ||
| private int timeLimit = 60; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이부분 전 캐치 못하고있었는뎁 감사합니다 ~!
| import io.f1.backend.domain.game.dto.response.GameStartResponse; | ||
| import io.f1.backend.domain.game.dto.response.QuestionStartResponse; | ||
|
|
||
| public record GameStartData( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이건 어디서 쓰이는걸까요?.?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어머 삭제했는데,, 충돌 해결하면서 제대로 안 사라졌나봅니다... 지우겠습니닷 !
| ofPlayerEvent(chatMessage.nickname(), RoomEventType.ENTER)); | ||
| ofPlayerEvent(chatMessage.nickname(), RoomEventType.CORRECT_ANSWER)); | ||
|
|
||
| timerService.cancelTimer(room); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[L2-변경협의]
cancelTimer 직전 시점에 timeout으로 인해 handleTimeout이 호출되는 edge case가 존재할 것 같습니다.
이 경우를 대비해 cancelTimer 메서드에 timer가 이미 정지된 상태인지 알려주는 boolean return 값을 부여하고,
이를 통해 조건문 처리를 하면 좋을 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cancelTimer() 호출 시점을 chat() 메서드 초반에 호출하는 방식으로 리팩토링해보겠습니다.
근데, timer가 이미 정지된 상태인지 알려주는 boolean return 값이 왜 필요한지 잘 모르겠는데, 혹시 더 설명해주실 수 있으실까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cancelTimer 호출 시 timer가 정지 상태라는 것은 handleTimeout이 먼저 호출됐다는 뜻일 겁니다.
그렇다면 handleTimeout 로직에 따라 Question Result와 System Notice가 브로드캐스트 된 이후 새로운 timer의 start와 Question Start를 진행하는 과정을 거치고 있을 텐데, chat 메서드 내의 cancelTimer 호출 이후 같은 로직을 진행하면 오동작을 일으킬 가능성이 존재할 것 같습니다.
따라서 boolean return 값을 통해 분기 처리를 해서 같은 로직을 진행하지 않도록 하는게 좋을 것 같다는 의견이었습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오.. 그렇군요 ! 이해했습니다. 리뷰를 읽고 생각을 해봤는데,, 채팅으로 정답을 맞혔을 때, 요청을 처리하는 도중 타임아웃이 된 경우를 생각하다보니,, 동시성까지 생각해야 해서 생각이 많아졌습니다..! 혹시 이 부분은 좀 더 생각해보고 다음 PR에 반영하도록 하겠습니다 !!! 감사합니다 ! :)
LimKangHyun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dev브랜치에 머지된 #90 번 PR 내용이 RoomService에 적용이 되어있지 않은것 같습니다!
어..! 왜지.. 이 부분 보고 수정하겠습니다 ! 감사합니다 ! |
| } else if (roomEventType == RoomEventType.CORRECT_ANSWER) { | ||
| message = " 님 정답입니다 !"; | ||
| } else if (roomEventType == RoomEventType.TIMEOUT) { | ||
| message = "땡 ~ ⏰ 제한 시간 초과!"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
공지 메시지로 쓰이는 문자열을 enum으로 관리하면 편할 것 같습니다 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인했습니다 ! 이 부분 수정해보겠습니다 !
LimKangHyun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다! 퀴즈때문에 이 부분도 언젠가는 수정이 필요하겠네요...
jiwon1217
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다 ! PR 내용이 알차네요 ㅎㅎ
sehee123
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다! PR 멋쪄요 .. 👀
🛰️ Issue Number
🪐 작업 내용
0. 타이머 소개
Room에 타이머를 추가했습니다.ScheduledExecutorService scheduler은 예약 작업을 실행시켜주는 스레드 서비스입니다.scheduler.schedule()을 호출하면, 일정 시간 이후에 특정 로직을 실행하도록 예약할 수 있습니다.ScheduledFuture<?> timer는 예약된 타이머 작업을 취소하거나 상태를 체크할 수 있게 해주는 객체입니다.scheduler로 예약해놓은 작업을 막아주기 위해timer.cancel()과 같이 타이머를 취소할 수 있습니다.TimerService로 분리하였습니다.1. 게임 시작 후 게임 진행 흐름 설명
1) 게임 시작
GAME_START와 함께,QUESTION_START로 첫 문제를 보내줍니다.QUESTION_START에는 서버가 응답을 보내주는 현재 시간 + 5초고, 클라이언트에서는 시간 보정 후, 해당 timestamp에 문제를 시작하게 됩니다.2) 게임 진행 + 타임 아웃
handleTimeout()메서드가 자동 호출됩니다.handleTimeout()에서는 한 문제가 종료되었기 때문에QUESTION_RESULT와SYSTEM_NOTICE를 브로드캐스팅해줍니다.QUESTION_START를 다시 브로드캐스팅합니다. (이어지는 문제이기 때문에, delay는 3초로 두었습니다.)3) 게임 진행 + 채팅으로 정답자있을 경우
handleTimeout이 실행되지 않도록!)QUESTION_START를 다시 브로드캐스팅합니다. (이어지는 문제이기 때문에, delay는 3초로 두었습니다.)4) RoomMapper에서 수정한 부분
RoomMapper에서 상수로 기본 시간 제한이 있고, 이걸 이용해서toGameSettingResponse를 만들더라구요 ! 그래서 GameSetting에 필드에 설정된 기본값이 사용되지 않는 것 같아 지워두었습니다.toQuestionResultResponse에서 파라미터로 ChatMessage를 받고 있었는데, 타임아웃의 경우ChatMessage정보가 없기 때문에, 파라미터를 String correctUser를 받도록 변경했습니다. -> 타임아웃 시, 닉네임 빈문자열2. 테스트 결과
1) 타임아웃
<서버>

<클라이언트>

2) 채팅으로 정답
<클라이언트>

<서버>

📚 Reference
✅ Check List